Perf - Migrate legacy img tags to next/image and update styles/config to support remote assets #2561
Conversation
…port remote assets
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaces plain HTML image usage with Next.js Image across several components, adds Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser
participant NextImage as next/image component
participant NextConfig as next.config.js
participant ImgSrc as External CDN (Gravatar / Sanity)
Browser->>NextImage: Render component (width/height/sizes)
NextImage->>NextConfig: Validate source domain
NextConfig->>ImgSrc: Request optimized image (if external)
ImgSrc-->>NextImage: Respond with image
NextImage-->>Browser: Hydrate/display optimized image
sequenceDiagram
participant Page as Page render
participant OldInline as Inline script (old)
participant ScriptComp as Next.js Script (new)
participant Window as window.__BUILD_INFO__
Page->>OldInline: (previous) inline script executes during render
OldInline->>Window: Sets __BUILD_INFO__
Note over Page,ScriptComp: Now using Script with beforeInteractive
Page->>ScriptComp: Render Script (beforeInteractive)
ScriptComp->>Window: Sets __BUILD_INFO__ earlier in load
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <div className={styles.imageContainer}> | ||
| {/* eslint-disable-next-line @next/next/no-img-element, jsx-a11y/alt-text */} | ||
| <img className={styles.image} src={getImageUrl(page.mainPhoto)} /> | ||
| <Image | ||
| className={styles.image} | ||
| src={getImageUrl(page.mainPhoto)} | ||
| alt="" | ||
| width={page.mainPhoto?.metadata?.dimensions?.width ?? 800} | ||
| height={page.mainPhoto?.metadata?.dimensions?.height ?? 600} | ||
| sizes="(max-width: 768px) 100vw, 800px" |
There was a problem hiding this comment.
<Image> when Sanity assets are disabled
getImageUrl(page.mainPhoto) returns an empty string when NEXT_PUBLIC_SANITY_PROJECT_ID is not set. <img> tags tolerated that and simply rendered nothing, but next/image will throw at runtime when the src is empty or points to an unconfigured host, causing the whole page to crash in local/dev environments where Sanity is off. Consider skipping the <Image> when getImageUrl returns a falsy value or providing a placeholder.
Useful? React with 👍 / 👎.
| const { _type: childElementType, _key: childElementKey, text } = childElement; | ||
| if (childElementType === 'inlineImage') { | ||
| const { asset, _key: imageKey } = childElement; | ||
| const width = asset?.metadata?.dimensions?.width ?? 800; | ||
| const height = asset?.metadata?.dimensions?.height ?? 600; | ||
| elementBlocks.push( | ||
| <div | ||
| key={`${bodyElementKey}-${childElementKey}-${imageKey}`} | ||
| className={styles.imageContainer} | ||
| > | ||
| {/* eslint-disable-next-line @next/next/no-img-element, jsx-a11y/alt-text */} | ||
| <img className={styles.image} src={getImageUrl(asset)} /> | ||
| <Image | ||
| className={styles.image} | ||
| src={getImageUrl(asset)} | ||
| alt="" | ||
| width={width} | ||
| height={height} | ||
| sizes="(max-width: 768px) 100vw, 800px" | ||
| /> |
There was a problem hiding this comment.
<Image> when no URL is available
Here getImageUrl(asset) is passed directly to next/image. When Sanity is not configured, getImageUrl returns ''; unlike the previous <img> tag, next/image will throw because the src is empty. Any page containing inline images will fail to render in setups without Sanity credentials. A simple conditional render or fallback placeholder avoids the crash.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/components/Sanity/Blocks/index.tsx (1)
37-42: Guard logic correctly prevents Next.js Image crashes when Sanity is disabled.The early return when
imageUrlis falsy ensures that pages won't crash whengetImageUrl(asset)returns an empty string (e.g., whenNEXT_PUBLIC_SANITY_PROJECT_IDis not configured). This resolves the concern from the previous review.src/components/Sanity/Page/index.tsx (1)
60-60: Guard logic correctly prevents Next.js Image crashes when Sanity is disabled.The dual check
page.mainPhoto && imageUrlensures the Image component is only rendered when both the asset object exists andgetImageUrlreturns a valid URL. This prevents runtime errors when Sanity is not configured, addressing the concern from the previous review.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/components/Sanity/Blocks/index.tsx(2 hunks)src/components/Sanity/Page/index.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.*
📄 CodeRabbit inference engine (.cursor/rules/bug-handling-with-todo-comments.mdc)
If you encounter a bug in existing code, or the instructions lead to suboptimal or buggy code, add comments starting with "TODO:" outlining the problems.
**/*.*: Utilize Early Returns: Use early returns to avoid nested conditions and improve readability.
Conditional Classes: Prefer conditional classes over ternary operators for class attributes.
**/*.*: Use comments sparingly, and when you do, make them meaningful.
Don't comment on obvious things. Excessive or unclear comments can clutter the codebase and become outdated.
Use comments to convey the 'why' behind specific actions or explain unusual behavior and potential pitfalls.
Provide meaningful information about the function's behavior and explain unusual behavior and potential pitfalls.
**/*.*: Write short functions that only do one thing.
Follow the single responsibility principle (SRP), which means that a function should have one purpose and perform it effectively.
If a function becomes too long or complex, consider breaking it into smaller, more manageable functions.Order functions with those that are composing other functions appearing earlier in the file. For example, if you have a menu with multiple buttons, define the menu function above the buttons.
**/*.*: Always add helpful comments to the code explaining what you are doing.
Never delete old comments, unless they are no longer relevant because the code has been rewritten or deleted.
**/*.*: Choose names for variables, functions, and classes that reflect their purpose and behavior.
A name should tell you why it exists, what it does, and how it is used. If a name requires a comment, then the name does not reveal its intent.
Use specific names that provide a clearer understanding of what the variables represent and how they are used.
Files:
src/components/Sanity/Page/index.tsxsrc/components/Sanity/Blocks/index.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/next-js-conventions.mdc)
**/*.{js,jsx,ts,tsx}: Rely on Next.js Pages Router for state changes.
Minimize 'use client' usage: Prefer server components and Next.js SSR features.
Minimize 'use client' usage: Use 'use client' only for Web API access in small components.
Minimize 'use client' usage: Avoid using 'use client' for data fetching or state management.
**/*.{js,jsx,ts,tsx}: Optimize Web Vitals (LCP, CLS, FID)
Use dynamic loading for non-critical components using @src/components/dls/Spinner/Spinner.tsx
Files:
src/components/Sanity/Page/index.tsxsrc/components/Sanity/Blocks/index.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react-functional-components.mdc)
src/components/**/*.tsx: Always use React functional components with hooks.
Use React.FC for functional components with props.
Files:
src/components/Sanity/Page/index.tsxsrc/components/Sanity/Blocks/index.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{tsx,jsx}: Use functional components over class components
Keep components small and focused
Extract reusable logic into custom hooks
Use composition over inheritance
Split large components into smaller, focused ones
Follow the Rules of Hooks
Use custom hooks for reusable logic
Use appropriate dependency arrays in useEffect
Implement cleanup in useEffect when needed
Avoid nested hooks
Use useState for local component state
Avoid prop drilling through proper state management
Implement proper memoization (useMemo, useCallback)
Use React.memo for expensive components
Avoid unnecessary re-renders
Implement proper lazy loading
Use proper key props in lists
Profile and optimize render performance
Show appropriate loading and error states
Handle async errors properly
Show user-friendly error messages
Implement proper fallback UI
Log errors appropriately
Handle edge cases gracefully
Use semantic HTML elements
Implement proper ARIA attributes
Ensure keyboard navigation
Handle focus management
Provide proper alt text for images
Use proper imports/exports
Document complex component logic
Files:
src/components/Sanity/Page/index.tsxsrc/components/Sanity/Blocks/index.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Implement proper prop types with TypeScript
**/*.tsx: Prefix interfaces for React props with 'Props' (e.g., ButtonProps)
Implement proper error boundariesReact: use functional components only; extract reusable logic into custom hooks; apply proper memoization (React.memo/useMemo/useCallback)
Files:
src/components/Sanity/Page/index.tsxsrc/components/Sanity/Blocks/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Use xstate for complex state logic
**/*.{ts,tsx}: TypeScript: prefer interfaces over type aliases for object shapes; avoid any; add explicit return types for public functions
Error handling: define and use custom error types; use async/await correctly; show user-friendly error messages
Use Redux Toolkit for state management and XState for complex state machines
Implement i18n with next-translate in components and utilities
Files:
src/components/Sanity/Page/index.tsxsrc/components/Sanity/Blocks/index.tsx
**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx,d.ts}: Prefer interfaces over types for object definitions
Use type for unions, intersections, and mapped types
Avoid usingany, preferunknownfor unknown types
Leverage TypeScript's built-in utility types
Use generics for reusable type patterns
Use PascalCase for type names and interfaces
Use camelCase for variables and functions
Use UPPER_CASE for constants
Use descriptive names with auxiliary verbs (e.g., isLoading, hasError)
Use explicit return types for public functions
Use arrow functions for callbacks and methods
Implement proper error handling with custom error types
Use function overloads for complex type scenarios
Prefer async/await over Promises
Use readonly for immutable properties
Leverage discriminated unions for type safety
Use type guards for runtime type checking
Implement proper null checking
Avoid type assertions unless necessary
Create custom error types for domain-specific errors
Use Result types for operations that can fail
Use try-catch blocks with typed catch clauses
Handle Promise rejections properly
Files:
src/components/Sanity/Page/index.tsxsrc/components/Sanity/Blocks/index.tsx
{src,types}/**/*.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript throughout the codebase with strict configuration
Files:
src/components/Sanity/Page/index.tsxsrc/components/Sanity/Blocks/index.tsx
src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer React functional components for UI implementation
Files:
src/components/Sanity/Page/index.tsxsrc/components/Sanity/Blocks/index.tsx
{src,types}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use path aliases @/ for src and @/dls/* for design system imports
Files:
src/components/Sanity/Page/index.tsxsrc/components/Sanity/Blocks/index.tsx
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Organize React components by feature and place DLS components under src/components/dls
Files:
src/components/Sanity/Page/index.tsxsrc/components/Sanity/Blocks/index.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Order imports: React first, then external, then internal; alphabetize within groups with blank lines between groups
Naming: camelCase for variables/functions; PascalCase for types, interfaces, and React components; UPPER_CASE for constants
Functions should be <= 30 lines, prefer early returns, single responsibility, and descriptive names
Files:
src/components/Sanity/Page/index.tsxsrc/components/Sanity/Blocks/index.tsx
**/*.{ts,tsx,js,jsx,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier formatting: single quotes, 2-space indentation, 100-character line width, and trailing commas
Files:
src/components/Sanity/Page/index.tsxsrc/components/Sanity/Blocks/index.tsx
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .cursor/rules/next-js-configuration-rule.mdc:0-0
Timestamp: 2025-08-01T08:49:55.662Z
Learning: Applies to next.config.js : Ensure the Next.js configuration is optimized for performance.
Learnt from: rrrokhtar
Repo: quran/quran.com-frontend-next PR: 2521
File: next.config.js:48-49
Timestamp: 2025-10-08T18:45:09.477Z
Learning: In the quran.com-frontend-next repository's next.config.js, generic image domains like `s3.amazonaws.com`, `lh3.googleusercontent.com`, and `platform-lookaside.fbsbx.com` are intentionally used to support avatars from external OAuth providers (Google, Facebook) and S3, where specific bucket names cannot be predetermined.
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-08-01T08:50:54.518Z
Learning: Applies to **/*.{tsx,jsx} : Provide proper alt text for images
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .cursor/rules/next-js-conventions.mdc:0-0
Timestamp: 2025-08-01T08:50:03.171Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Minimize 'use client' usage: Prefer server components and Next.js SSR features.
📚 Learning: 2025-08-01T08:50:54.518Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-08-01T08:50:54.518Z
Learning: Applies to **/*.{tsx,jsx} : Provide proper alt text for images
Applied to files:
src/components/Sanity/Page/index.tsxsrc/components/Sanity/Blocks/index.tsx
📚 Learning: 2025-08-01T08:50:03.171Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .cursor/rules/next-js-conventions.mdc:0-0
Timestamp: 2025-08-01T08:50:03.171Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Rely on Next.js Pages Router for state changes.
Applied to files:
src/components/Sanity/Page/index.tsx
📚 Learning: 2025-10-07T08:38:28.343Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T08:38:28.343Z
Learning: Applies to **/*.{ts,tsx} : Implement i18n with next-translate in components and utilities
Applied to files:
src/components/Sanity/Page/index.tsx
📚 Learning: 2025-10-08T18:45:09.477Z
Learnt from: rrrokhtar
Repo: quran/quran.com-frontend-next PR: 2521
File: next.config.js:48-49
Timestamp: 2025-10-08T18:45:09.477Z
Learning: In the quran.com-frontend-next repository's next.config.js, generic image domains like `s3.amazonaws.com`, `lh3.googleusercontent.com`, and `platform-lookaside.fbsbx.com` are intentionally used to support avatars from external OAuth providers (Google, Facebook) and S3, where specific bucket names cannot be predetermined.
Applied to files:
src/components/Sanity/Blocks/index.tsx
🧬 Code graph analysis (2)
src/components/Sanity/Page/index.tsx (1)
src/lib/sanity.ts (1)
getImageUrl(33-38)
src/components/Sanity/Blocks/index.tsx (1)
src/lib/sanity.ts (1)
getImageUrl(33-38)
🔇 Additional comments (3)
src/components/Sanity/Blocks/index.tsx (1)
2-2: LGTM: Next.js Image import added correctly.The import of the Next.js Image component is properly placed and will enable automatic optimization.
src/components/Sanity/Page/index.tsx (2)
4-4: LGTM: Next.js Image import added correctly.The import of the Next.js Image component is properly placed and consistent with the migration strategy.
30-30: Good extraction of imageUrl for conditional rendering.Precomputing
imageUrlat the top of the component enables clean conditional logic downstream.
| <Image | ||
| className={styles.image} | ||
| src={imageUrl} | ||
| alt="" | ||
| width={width} | ||
| height={height} | ||
| sizes="(max-width: 768px) 100vw, 800px" | ||
| /> |
There was a problem hiding this comment.
Provide meaningful alt text for accessibility.
The empty alt="" violates accessibility guidelines. Sanity assets may include descriptive metadata; if asset.alt or a similar field exists, use it. Otherwise, derive descriptive text from context (e.g., the surrounding content or page title).
Apply this diff if asset.alt is available:
<Image
className={styles.image}
src={imageUrl}
- alt=""
+ alt={asset.alt || 'Inline image'}
width={width}
height={height}
sizes="(max-width: 768px) 100vw, 800px"
/>Based on learnings.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/Sanity/Blocks/index.tsx around lines 48 to 55, the Image
component currently sets alt="" which breaks accessibility; update the code to
pass a meaningful alt string: use the Sanity asset's alt field (e.g. asset.alt
or image.asset?.alt) if present, otherwise derive a fallback (caption, block
heading, or page title) and use that as the alt prop; ensure you never leave alt
empty and keep it as an empty string only for purely decorative images —
otherwise supply the descriptive fallback.
| <Image | ||
| className={styles.image} | ||
| src={imageUrl} | ||
| alt="" | ||
| width={page.mainPhoto?.metadata?.dimensions?.width ?? 800} | ||
| height={page.mainPhoto?.metadata?.dimensions?.height ?? 600} | ||
| sizes="(max-width: 768px) 100vw, 800px" | ||
| /> |
There was a problem hiding this comment.
Provide meaningful alt text for accessibility.
The empty alt="" violates accessibility guidelines. Sanity's page schema likely includes descriptive metadata for mainPhoto. Use page.mainPhoto.alt or derive descriptive text from page.title.
Apply this diff if page.mainPhoto.alt is available:
<Image
className={styles.image}
src={imageUrl}
- alt=""
+ alt={page.mainPhoto.alt || page.title || 'Page hero image'}
width={page.mainPhoto?.metadata?.dimensions?.width ?? 800}
height={page.mainPhoto?.metadata?.dimensions?.height ?? 600}
sizes="(max-width: 768px) 100vw, 800px"
/>Based on learnings.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Image | |
| className={styles.image} | |
| src={imageUrl} | |
| alt="" | |
| width={page.mainPhoto?.metadata?.dimensions?.width ?? 800} | |
| height={page.mainPhoto?.metadata?.dimensions?.height ?? 600} | |
| sizes="(max-width: 768px) 100vw, 800px" | |
| /> | |
| <Image | |
| className={styles.image} | |
| src={imageUrl} | |
| alt={page.mainPhoto?.alt || page.title || 'Page hero image'} | |
| width={page.mainPhoto?.metadata?.dimensions?.width ?? 800} | |
| height={page.mainPhoto?.metadata?.dimensions?.height ?? 600} | |
| sizes="(max-width: 768px) 100vw, 800px" | |
| /> |
🤖 Prompt for AI Agents
In src/components/Sanity/Page/index.tsx around lines 62 to 69, the Image
component currently uses an empty alt (alt="") which breaks accessibility;
replace it with a meaningful value by using page.mainPhoto?.alt when present,
otherwise fall back to a derived string such as `Main photo for ${page.title ??
'page'}` (or a shorter descriptive fallback), ensuring you safely handle
undefined page/title so the prop is always a string.
Summary
Swapped the homepage Sanity images to use Next’s![]()
so they load faster, stay responsive, and get automatic lazy loading from the CDN.
Moved the build info script into next/script so it doesn’t block the first paint.
Type of change
Test plan
I've checked with MCP Playwright that the changes took effect on the site.
Checklist
Summary by CodeRabbit
New Features
Performance
Style